Skip to content

fix(github): create domain accounts for non-committer authors (#8886) - #8894

Merged
klesh merged 2 commits into
apache:mainfrom
JAORMX:fix/github-orphan-accounts-8886
Jun 12, 2026
Merged

fix(github): create domain accounts for non-committer authors (#8886)#8894
klesh merged 2 commits into
apache:mainfrom
JAORMX:fix/github-orphan-accounts-8886

Conversation

@JAORMX

@JAORMX JAORMX commented May 30, 2026

Copy link
Copy Markdown
Contributor

Problem

issues.creator_id and pull_requests.author_id are written for every author, but a domain accounts row is only created for users we collected a full profile for (effectively, committers). Authors who only filed an issue, or opened a PR without committing, become orphan foreign keys:

SELECT i.url, i.creator_id, i.creator_name, a.user_name
  FROM issues i
  LEFT JOIN accounts a ON a.id = i.creator_id
 WHERE i.url = 'https://github.com/stacklok/toolhive/issues/4297';

creator_id is set and creator_name is milichev, but a.user_name is NULL. Bot filters keying on accounts.user_name LIKE '%[bot]' miss those rows for the same reason.

Fixes #8886.

Root cause

The issue and PR extractors already record every author in _tool_github_repo_accounts. But ConvertAccounts sourced the domain accounts table FROM _tool_github_accounts, which is only populated for users we fetched full profiles for. So the convertors generate creator_id / author_id for everyone, while accounts only ever gets the committers.

Change

  • ConvertAccounts now reads FROM _tool_github_repo_accounts LEFT JOIN _tool_github_accounts, so every user the repo references becomes a domain account: enriched with name/email/avatar when we have the detail, login-only otherwise. The domain id uses the same didgen generator the issue/PR convertors use, so the foreign keys line up.
  • pr_extractor also emits a repo_account for the merged_by user, which fixes pull_requests.merged_by_id (that user wasn't recorded anywhere before).
  • The query is MySQL/PostgreSQL-agnostic (COALESCE not IFNULL, no backtick quoting, values parameterized via the dal). The join mirrors the existing one in account_org_collector.go.

Testing

  • The e2e fixture didn't exercise the bug (every referenced account already had a profile row), so I added the orphan case from the issue: milichev, referenced by the repo with no detail row. It now gets a login-only accounts row.
  • Added a referential-integrity assertion to TestAccountDataFlow: every account the repo references must resolve to a domain accounts row, generated with the same id generator the convertors use. Verified it fails against the old converter and passes with the fix.
  • Full plugins/github/e2e suite passes on both MySQL and PostgreSQL.

Out of scope

Plural issue/PR assignees and PR requested reviewers aren't seeded into _tool_github_repo_accounts, so those FKs can still be unresolved. The github_graphql plugin likely shares the root cause. Happy to follow up on these separately.

🤖 Generated with Claude Code

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. component/plugins This issue or PR relates to plugins pr-type/bug-fix This PR fixes a bug labels May 30, 2026
github:GithubAccount:1:7496278,i@andypan.me,Andy Pan,panjf2000,https://avatars.githubusercontent.com/u/7496278?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,5,
github:GithubAccount:1:8518239,badger@gitter.im,The Gitter Badger,gitter-badger,https://avatars.githubusercontent.com/u/8518239?v=4,,"{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,9,
github:GithubAccount:1:964542,sarath.sp06@gmail.com,Sarath Sadasivan Pillai,sarathsp06,https://avatars.githubusercontent.com/u/964542?v=4,"exotel,leadmrktr,shellagilehub,odysseyhack,boodltech","{""ConnectionId"":1,""Name"":""panjf2000/ants""}",_raw_github_api_accounts,1,
github:GithubAccount:1:1052632,runner.mei@,runner,runner-mei,https://avatars.githubusercontent.com/u/1052632?v=4,,,,0,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_raw_data fields are missing, please fix it. Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, will do when I'm back on my computer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in b67aad7! The converter now carries the _raw_data fields over: it prefers the enriched _tool_github_accounts row when we collected a profile, and falls back to the _tool_github_repo_accounts row for the non-committer case (that's why milichev shows _raw_github_api_issues provenance). The e2e fixture was missing the _raw_data columns too, so I added those and regenerated the snapshot... the enriched rows now carry exactly the same provenance as before this PR.

While at it, I also guarded pr_convertor.go against generating account ids for a zero AuthorId/MergedById. An unmerged PR would otherwise get merged_by_id pointing at github:GithubAccount:1:0, which is the same orphan shape this PR is fixing (issue_convertor already had that guard).

Verified the full github e2e suite on both MySQL and PostgreSQL.

JAORMX and others added 2 commits June 11, 2026 19:55
…#8886)

ConvertAccounts sourced the domain `accounts` table FROM _tool_github_accounts,
which is only populated for users we collected full profiles for (effectively,
committers). Issue and PR authors who never committed were written into
_tool_github_repo_accounts but never converted, so issues.creator_id and
pull_requests.author_id pointed at accounts rows that didn't exist.

Source ConvertAccounts FROM _tool_github_repo_accounts LEFT JOIN
_tool_github_accounts instead, so every user the repo references gets a domain
account, enriched with profile detail when we have it and login-only otherwise.
The domain id uses the same generator the issue/PR convertors use, so the FKs
line up. Also emit a repo_account for a PR's merged_by user so
pull_requests.merged_by_id resolves too.

The query stays MySQL/PostgreSQL-agnostic (COALESCE, no backtick quoting,
parameterized via the dal) and mirrors the join already in
account_org_collector.go.

Adds the non-committer orphan case to the e2e fixture plus a referential-
integrity assertion in TestAccountDataFlow. Verified on both MySQL and
PostgreSQL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review feedback on apache#8894: the rewritten ConvertAccounts dropped the
_raw_data fields from converted accounts. Select them with COALESCE,
preferring the enriched _tool_github_accounts row and falling back to
the _tool_github_repo_accounts row for non-committers. The e2e fixture
now carries the _raw_data columns like every other tool fixture, and
the regenerated snapshot pins the pre-existing provenance for enriched
accounts plus the issue-extractor provenance for the non-committer
case.

Also guard PR author and merged-by id generation against zero ids: an
unmerged PR or a deleted user otherwise yields the domain id
github:GithubAccount:<conn>:0, the same orphan-FK shape this PR fixes.
issue_convertor already guards its AuthorId the same way.

Verified with the full github e2e suite on both MySQL and PostgreSQL.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JAORMX
JAORMX force-pushed the fix/github-orphan-accounts-8886 branch from b5805d5 to b67aad7 Compare June 11, 2026 17:16

@klesh klesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Thanks for our contribution.

@klesh
klesh merged commit f488841 into apache:main Jun 12, 2026
10 checks passed
bujjibabukatta pushed a commit to bujjibabukatta/incubator-devlake that referenced this pull request Jun 18, 2026
…#8886) (apache#8894)

* fix(github): create domain accounts for non-committer authors (apache#8886)

ConvertAccounts sourced the domain `accounts` table FROM _tool_github_accounts,
which is only populated for users we collected full profiles for (effectively,
committers). Issue and PR authors who never committed were written into
_tool_github_repo_accounts but never converted, so issues.creator_id and
pull_requests.author_id pointed at accounts rows that didn't exist.

Source ConvertAccounts FROM _tool_github_repo_accounts LEFT JOIN
_tool_github_accounts instead, so every user the repo references gets a domain
account, enriched with profile detail when we have it and login-only otherwise.
The domain id uses the same generator the issue/PR convertors use, so the FKs
line up. Also emit a repo_account for a PR's merged_by user so
pull_requests.merged_by_id resolves too.

The query stays MySQL/PostgreSQL-agnostic (COALESCE, no backtick quoting,
parameterized via the dal) and mirrors the join already in
account_org_collector.go.

Adds the non-committer orphan case to the e2e fixture plus a referential-
integrity assertion in TestAccountDataFlow. Verified on both MySQL and
PostgreSQL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(github): carry _raw_data provenance and guard zero account ids

Review feedback on apache#8894: the rewritten ConvertAccounts dropped the
_raw_data fields from converted accounts. Select them with COALESCE,
preferring the enriched _tool_github_accounts row and falling back to
the _tool_github_repo_accounts row for non-committers. The e2e fixture
now carries the _raw_data columns like every other tool fixture, and
the regenerated snapshot pins the pre-existing provenance for enriched
accounts plus the issue-extractor provenance for the non-committer
case.

Also guard PR author and merged-by id generation against zero ids: an
unmerged PR or a deleted user otherwise yields the domain id
github:GithubAccount:<conn>:0, the same orphan-FK shape this PR fixes.
issue_convertor already guards its AuthorId the same way.

Verified with the full github e2e suite on both MySQL and PostgreSQL.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kpiwko added a commit to kpiwko/devlake that referenced this pull request Jul 16, 2026
Both were already fixed upstream in apache#8894 (fixes apache#8886)
but this fork had drifted from it:

- pr_convertor.go: guard AuthorId/MergedById against 0 before
  generating a domain account ID. Without it, a null author/merged_by
  from the REST API produces a phantom github:GithubAccount:X:0 ID
  that matches no accounts row. Confirmed byte-identical to upstream's
  current code.
- pr_extractor.go: restore the merged_by repo_account emission this
  fork had dropped, so pull_requests.merged_by_id resolves to a real
  account. Confirmed byte-identical to upstream's current code.

DPROD-1342

Co-Authored-By: Claude <noreply@anthropic.com>
kpiwko added a commit to kpiwko/devlake that referenced this pull request Jul 16, 2026
…populate IsBot

Adopts apache#8894's unified ConvertAccounts query (FROM
_tool_github_repo_accounts LEFT JOIN _tool_github_accounts) instead of
maintaining this fork's own two-pass design (ConvertAccounts +
convertOrphanedRepoAccounts, added by cd99281 for the same problem).
Upstream's version is more complete (correct _raw_data provenance,
which the two-pass patch had broken) and removes a large source of
rebase conflict risk.

IsBot is populated via isBotAccount() (API Type=="Bot" or well-known
login conventions: [bot]/-bot/-robot suffixes, copilot/dependabot/
github-actions/codecov-commenter logins) plus hasNoProfileData() — an
account with no avatar_url ever collected (real GitHub users always
get one) is almost always a bot that login/type patterns missed. This
replaces the old orphan-pass's blanket "orphans are always bots"
heuristic with an equivalent signal inside the single query.

Validated against live data: closes the exact gap DPROD-1262
described (konflux-ci-triage[bot] and konflux-ci-review[bot], both
previously missing from accounts entirely, now resolve with
IsBot=true) and confirmed on a fresh konflux-ci/devlake collection
(codecov-commenter correctly flagged despite the GitHub API reporting
it as Type: "User" with a full profile).

DPROD-1262
DPROD-1342

Co-Authored-By: Claude <noreply@anthropic.com>
kpiwko added a commit to kpiwko/devlake that referenced this pull request Jul 16, 2026
Materializes docs/research/bot-commit-identification.md into the
working tree (previously only existed on the unmerged PR konflux-ci#118 branch)
and revises it to reflect adopting apache#8894 instead of
extending this fork's own two-pass account_convertor design, plus the
codecov-commenter finding from live testing.

Also backfills docs/upstream-diffs.md with the account_convertor.go
divergence this fork already carried (introduced by cd99281,
DPROD-1259, never logged) alongside the new IsBot-specific entries.
pr_convertor.go and pr_extractor.go now match upstream exactly and
need no entry going forward.

DPROD-1342

Co-Authored-By: Claude <noreply@anthropic.com>
kpiwko added a commit to kpiwko/devlake that referenced this pull request Aug 4, 2026
Both were already fixed upstream in apache#8894 (fixes apache#8886)
but this fork had drifted from it:

- pr_convertor.go: guard AuthorId/MergedById against 0 before
  generating a domain account ID. Without it, a null author/merged_by
  from the REST API produces a phantom github:GithubAccount:X:0 ID
  that matches no accounts row. Confirmed byte-identical to upstream's
  current code.
- pr_extractor.go: restore the merged_by repo_account emission this
  fork had dropped, so pull_requests.merged_by_id resolves to a real
  account. Confirmed byte-identical to upstream's current code.

DPROD-1342

Co-Authored-By: Claude <noreply@anthropic.com>
kpiwko added a commit to kpiwko/devlake that referenced this pull request Aug 4, 2026
…populate IsBot

Adopts apache#8894's unified ConvertAccounts query (FROM
_tool_github_repo_accounts LEFT JOIN _tool_github_accounts) instead of
maintaining this fork's own two-pass design (ConvertAccounts +
convertOrphanedRepoAccounts, added by cd99281 for the same problem).
Upstream's version is more complete (correct _raw_data provenance,
which the two-pass patch had broken) and removes a large source of
rebase conflict risk.

IsBot is populated via isBotAccount() (API Type=="Bot" or well-known
login conventions: [bot]/-bot/-robot suffixes, copilot/dependabot/
github-actions/codecov-commenter logins) plus hasNoProfileData() — an
account with no avatar_url ever collected (real GitHub users always
get one) is almost always a bot that login/type patterns missed. This
replaces the old orphan-pass's blanket "orphans are always bots"
heuristic with an equivalent signal inside the single query.

Validated against live data: closes the exact gap DPROD-1262
described (konflux-ci-triage[bot] and konflux-ci-review[bot], both
previously missing from accounts entirely, now resolve with
IsBot=true) and confirmed on a fresh konflux-ci/devlake collection
(codecov-commenter correctly flagged despite the GitHub API reporting
it as Type: "User" with a full profile).

DPROD-1262
DPROD-1342

Co-Authored-By: Claude <noreply@anthropic.com>
kpiwko added a commit to kpiwko/devlake that referenced this pull request Aug 4, 2026
Materializes docs/research/bot-commit-identification.md into the
working tree (previously only existed on the unmerged PR konflux-ci#118 branch)
and revises it to reflect adopting apache#8894 instead of
extending this fork's own two-pass account_convertor design, plus the
codecov-commenter finding from live testing.

Also backfills docs/upstream-diffs.md with the account_convertor.go
divergence this fork already carried (introduced by cd99281,
DPROD-1259, never logged) alongside the new IsBot-specific entries.
pr_convertor.go and pr_extractor.go now match upstream exactly and
need no entry going forward.

DPROD-1342

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/plugins This issue or PR relates to plugins pr-type/bug-fix This PR fixes a bug size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitHub plugin: domain accounts row missing for non-contributor authors, leaving issues.creator_id / pull_requests.author_id as orphan FKs

2 participants